Skip to content

t1491: fix Bash 3.2 bad substitution in config_get indirect expansion#4931

Merged
marcusquinn merged 1 commit intomainfrom
bugfix/t1491-bash32-config-helper
Mar 15, 2026
Merged

t1491: fix Bash 3.2 bad substitution in config_get indirect expansion#4931
marcusquinn merged 1 commit intomainfrom
bugfix/t1491-bash32-config-helper

Conversation

@alex-solovyev
Copy link
Copy Markdown
Collaborator

@alex-solovyev alex-solovyev commented Mar 15, 2026

Summary

  • Replace ${!env_var:-} with eval "env_val=\${$env_var:-}" at 3 locations in config-helper.sh (lines 333, 527, 732)
  • The ${!var:-default} indirect expansion syntax causes "bad substitution" on macOS /bin/bash 3.2.57
  • This caused config_get to fail silently during pulse-wrapper.sh startup, making MAX_WORKERS_CAP and QUALITY_DEBT_CAP_PCT fall back to hardcoded defaults instead of reading from settings.json

Verification

  • ShellCheck clean (zero violations)
  • Tested config_get with env var override set → returns override value
  • Tested config_get with env var unset → falls through to JSONC config value
  • No remaining ${!env_var:-} instances in the file

Closes #4929

Summary by CodeRabbit

Release Notes

  • Bug Fixes
    • Improved compatibility with older Bash versions for environment variable configuration handling.
    • Fixed issues with environment override initialization to prevent errors when configuration variables are not set.

Replace ${!env_var:-} with eval-based equivalent at 3 locations in
config-helper.sh. The ${!var:-default} syntax causes 'bad substitution'
on macOS /bin/bash 3.2.57, making config_get fail silently during
pulse-wrapper.sh startup.

Closes #4929
@gemini-code-assist
Copy link
Copy Markdown

Warning

You have reached your daily quota limit. Please wait up to 24 hours and I will start processing your requests again!

@github-actions github-actions bot added the bug Auto-created from TODO.md tag label Mar 15, 2026
@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai bot commented Mar 15, 2026

Caution

Review failed

Pull request was closed or merged during review

Walkthrough

Fixed Bash 3.2 compatibility issue in config-helper.sh by replacing unsupported ${!VAR:-} indirect expansion syntax with eval-based approach at three locations. Explicitly initialized env_val variable and updated comments to document compatibility rationale.

Changes

Cohort / File(s) Summary
Bash 3.2 Compatibility Fix
.agents/scripts/config-helper.sh
Replaced ${!env_var:-} with eval-based indirect expansion at lines 333, 527, and 732 to support Bash 3.2. Added explicit env_val initialization and documentation comments explaining the Bash 4.0+ incompatibility workaround.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐚 Three lines of Bash once caused a fright,
${!VAR:-} broke on macOS night—
With eval and care, we've set it right,
Now Bash 3.2 runs clean and bright! ✨

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately reflects the main change: fixing Bash 3.2 compatibility by replacing incompatible indirect expansion syntax in config_get.
Linked Issues check ✅ Passed All objectives from issue #4929 are met: three occurrences of ${!env_var:-} replaced with eval-based Bash 3.2-compatible syntax, ShellCheck verified clean, config_get behavior preserved for both set and unset env vars.
Out of Scope Changes check ✅ Passed All changes are directly scoped to fixing Bash 3.2 compatibility in config-helper.sh; no unrelated modifications or scope creep detected.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
  • 📝 Generate docstrings (stacked PR)
  • 📝 Generate docstrings (commit on current branch)
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment
  • Commit unit tests in branch bugfix/t1491-bash32-config-helper
📝 Coding Plan
  • Generate coding plan for human review comments

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@marcusquinn
Copy link
Copy Markdown
Owner

Duplicate PR detected: PR #4930 (branch bugfix/issue-4929-bash32-config-get, author: marcusquinn) also targets issue #4929 and was opened 9 seconds earlier.

PR #4931 (this PR) is more comprehensive — fixes 3 locations in config-helper.sh vs PR #4930's 1 location. Recommend: close PR #4930 and merge this one once CI passes.

Supervisor will check CI results next cycle and close the smaller PR if this one passes.

@github-actions
Copy link
Copy Markdown
Contributor

🔍 Code Quality Report

�[0;35m[MONITOR]�[0m Code Review Monitoring Report

�[0;34m[INFO]�[0m Latest Quality Status:
SonarCloud: 0 bugs, 0 vulnerabilities, 362 code smells

�[0;34m[INFO]�[0m Recent monitoring activity:
Sun Mar 15 05:25:07 UTC 2026: Code review monitoring started
Sun Mar 15 05:25:08 UTC 2026: SonarCloud - Bugs: 0, Vulnerabilities: 0, Code Smells: 362

📈 Current Quality Metrics

  • BUGS: 0
  • CODE SMELLS: 362
  • VULNERABILITIES: 0

Generated on: Sun Mar 15 05:25:10 UTC 2026


Generated by AI DevOps Framework Code Review Monitoring

@sonarqubecloud
Copy link
Copy Markdown

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bug Auto-created from TODO.md tag review-feedback-scanned Merged PR already scanned for quality feedback

Projects

None yet

Development

Successfully merging this pull request may close these issues.

t1491: fix: Bash 3.2 bad substitution in config_get indirect expansion

2 participants